Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kotlin.native.cocoapods.targets can be a custom list of architectures #3861

Merged
merged 3 commits into from
Nov 4, 2020
Merged

kotlin.native.cocoapods.targets can be a custom list of architectures #3861

merged 3 commits into from
Nov 4, 2020

Conversation

MikeKulasinski-visa
Copy link
Contributor

As sdk developers we would like to have possibility to specify a concrete list of architectures to build as fat framework. When clients of ours would like to use the framework they need to be able to run it on both iPhone and Simulator.

Therefore in this PR I am proposing a change that allows to dynamically specify a list of architectures with comma separated manner. Rest of the functionality stays untouched, meaning user still can provide just one target if they wish.

@crisredfi
Copy link

This is a key feature for us. For our company releases we require to pack everything into frameworks and we need to specify 3 architectures to include in the framework (32, 64 and X86_64). This PR would solve some headaches we have with custom scripts to achieve our needs.

@abelkov abelkov added the Native label Oct 20, 2020
@MikeKulasinski-visa
Copy link
Contributor Author

@abelkov @ilmat192 this relatively simple problem is blocking us from releasing our product, do you think it is possible to increase priority of it a bit? Thanks a lot

@ilmat192
Copy link
Contributor

LGTM but could you please also add a test to CocoaPodsIT for this feature? I think building a DEBUG framework for iosX64+iosArm64 will be enough to test it. You can use testCinteropExtraOpts or testCustomPackageName as a reference.

Feel free to ask me about the test infrastructure if you have any questions.

@MikeKulasinski-visa
Copy link
Contributor Author

@ilmat192 I have spent quite some time already for analyzing the tests. I have managed to run them and I think I understand what is happening there. So it seems that you are creating a temporary project on which you execute some tasks and compare results.

I have not found a single test that tests current behaviour of targets, like a test confirming that iphoneos option is producing IOS_ARM32, IOS_ARM64` architectures that are packed in one fat framework. Therefore I can't really tell how I should write this test.

So I need a little bit more help since I am not quite sure how to do it.

@MikeKulasinski-visa
Copy link
Contributor Author

@ilmat192 perhaps we can merge without test? Since original TARGETS were not tested anyway? I would normally took time to write the test, however in this situation it requires quite a lot of time to get on board with your testing setup.

@ilmat192
Copy link
Contributor

ilmat192 commented Nov 3, 2020

@MikeKulasinski-visa Sorry for the delay: I was sick and had no chance to answer.

You're right, the test machinery in the CocoaPods tests is a little bit complicated. So I've prepared a simple test for this change, you can include it to your PR: 193b4f2.

As for lack of tests for arm32+arm64 fat frameworks, we really don't test this case but this is a flaw that should be eventually fixed.

@MikeKulasinski-visa
Copy link
Contributor Author

@ilmat192 thanks for helping out. I had to rebase on top of master since there were conflicts with tests. Someone added a new test :) I hope it is ok if we have a different commit hash than the one you had?

ilmat192
ilmat192 approved these changes Nov 4, 2020
View reviewed changes
Copy link
Contributor

@ilmat192 ilmat192 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll wait until the CI run and merge this PR once it finishes.

@ilmat192
Copy link
Contributor

ilmat192 commented Nov 4, 2020

One more remark since you're going to distribute K/N modules as frameworks. Please note that including several K/N frameworks with dependent APIs into a single app may not work as you expect. The point is that Kotlin/Native follows the "closed world" compilation model. It means that all dependencies of a K/N module are packed into a single final framework.

Consider two Kotlin modules A and B both depending on a third module lib. Let lib contains only one class Foo. In this model, frameworks produced from A and B will contain their own copies of Foo (A.Foo and B.Foo in terms of Swift symbols). Thus if Foo is used in APIs of these modules they will not be able to interact with each other because each of them expects its own Foo.

Please take this into account when working on your SDK. Some additional info can be found in
JetBrains/kotlin-native#2423.

Note also that this restriction affects only frameworks with common dependencies other than the Kotlin stdlib. So Kotlin frameworks without common API dependencies work fine in a single application (see 1.3.70 blog post).

@ilmat192 ilmat192 merged commit bc2b396 into JetBrains:master Nov 4, 2020
@MikeKulasinski-visa
Copy link
Contributor Author

Thanks for the update :)

erokhins pushed a commit that referenced this pull request Nov 28, 2020
@vvlevchenko @erokhins
[kotlin compiler][update] 1.4.30-dev-2395
780c792 
* 0e8cf58 - (HEAD -> master, tag: build-1.4.30-dev-2395, origin/master, origin/HEAD) Minor: reformat (vor 16 Stunden) <Pavel Kirpichenkov>
* b79b94f - [MPP] Allow smart casts for properties from dependsOn modules (vor 16 Stunden) <Pavel Kirpichenkov>
* 778bbd7 - [MPP] Add test for KT-42754 (vor 16 Stunden) <Pavel Kirpichenkov>
* 9f0cec3 - (tag: build-1.4.30-dev-2389) Native, Gradle IT: Add verbosity to IT (vor 2 Tagen) <Dmitriy Dolovov>
* 9adc1a6 - (tag: build-1.4.30-dev-2381) JVM IR: generate 'main' wrappers as non-final (vor 2 Tagen) <Alexander Udalov>
* 791be7c - JVM IR: improve ABI of properties in multifile facades (vor 2 Tagen) <Alexander Udalov>
* 55974b4 - JVM IR: do not generate InlineOnly property accessors in multifile facade (vor 2 Tagen) <Alexander Udalov>
* 500b1cf - JVM IR: fix flags of $default methods in multi-file facades (vor 2 Tagen) <Alexander Udalov>
* d326d6a - Specify module name via moduleName option instead of freeCompilerArgs (vor 2 Tagen) <Alexander Udalov>
* bdb8a58 - JVM IR: merge JvmBackendFacade into JvmIrCodegenFactory (vor 2 Tagen) <Alexander Udalov>
* 5c2c259 - Minor, rename file to keep git history (vor 2 Tagen) <Alexander Udalov>
* 624204c - JVM IR: remove outdated code in JvmIrCodegenFactory (vor 2 Tagen) <Alexander Udalov>
* b2d49e9 - Minor, fix whitespace in toString (vor 2 Tagen) <Alexander Udalov>
* c1a292b - Psi2ir: improve exception stack traces (vor 2 Tagen) <Alexander Udalov>
* 3cb202f - (tag: build-1.4.30-dev-2373) Gradle IT: Fix Gradle output checks in Kotlin/Native ITs (vor 3 Tagen) <Dmitriy Dolovov>
* e3db963 - (tag: build-1.4.30-dev-2361) Add javadoc and sources artifacts to kotlin-gradle-build-metrics module (vor 3 Tagen) <Alexander Likhachev>
* 5f2a963 - Better wording and comments for klib compatibility code (vor 3 Tagen) <Alexander Gorshenev>
* 891a4c4 - Dropped outdated klib version compatibility mechanisms (vor 3 Tagen) <Alexander Gorshenev>
* 8b2b36d - Enabled klib abi version compatibility (vor 3 Tagen) <Alexander Gorshenev>
* e434a1c - (tag: build-1.4.30-dev-2355) FIR: Drop unused AbstractFirOverrideScope::create*Copy (vor 3 Tagen) <Denis Zharkov>
* 2e4c41c - FIR: Drop PossiblyFirFakeOverrideSymbol (vor 3 Tagen) <Denis Zharkov>
* 7b48625 - FIR: Remove FirCallableSymbol::overriddenSymbol (vor 3 Tagen) <Denis Zharkov>
* 78b374e - FIR: Do not set overriddenSymbol for KFunction::invoke (vor 3 Tagen) <Denis Zharkov>
* a11b488 - FIR: Add workaround for combination of intersection + delegated members (vor 3 Tagen) <Denis Zharkov>
* 41f878e - FIR: Adjust test data for type alias constructors (vor 3 Tagen) <Denis Zharkov>
* 96c3436 - FIR: Temporary adjust diagnostics test data (vor 3 Tagen) <Denis Zharkov>
* d58e66e - FIR: Merge both synthetic type alias constructor kinds (vor 3 Tagen) <Denis Zharkov>
* f9aab77 - FIR: Simplify ConstructorProcessing relevant to type aliases (vor 3 Tagen) <Denis Zharkov>
* 8a949b0 - FIR: Use attribute instead of overriddenSymbol for synthetic type alias constructor (vor 3 Tagen) <Denis Zharkov>
* 4282e27 - FIR: Get rid of usages of FirCallableSymbol::overriddenSymbol (vor 3 Tagen) <Denis Zharkov>
* 728c2a8 - FIR: Do not propagate overriddenSymbol at FirObjectImportedCallableScope (vor 3 Tagen) <Denis Zharkov>
* 037ba4d - FIR: Simplify VariableStorage::isStable (vor 3 Tagen) <Denis Zharkov>
* 317d981 - FIR: Do not use overriddenSymbol for imported from object (vor 3 Tagen) <Denis Zharkov>
* 65119ad - FIR: Adjust test data. FakeOverride -> SubssitutionOverride (vor 3 Tagen) <Denis Zharkov>
* 3e37995 - FIR: Get rid of FirCallableSymbol::isFakeOverride and isIntersectionOverride (vor 3 Tagen) <Denis Zharkov>
* 36387d9 - (tag: build-1.4.30-dev-2323) Add IC metrics reporting (vor 3 Tagen) <Alexey Tsvetkov>
* 5bde645 - Metrics reporting utils (vor 3 Tagen) <Alexey Tsvetkov>
* 97a0968 - Remove CompositeICReporterAsync (vor 3 Tagen) <Alexey Tsvetkov>
* 2c1e4c1 - (tag: build-1.4.30-dev-2319) [IR] Use more specific type (IrSimpleFunction) for accessors in IrLocalDelegatedProperty (vor 3 Tagen) <Zalim Bashorov>
* 11bc1c0 - (tag: build-1.4.30-dev-2316) Revert "Fix configuration cache issue in AbstractKotlinTarget" (vor 3 Tagen) <Sergey Igushkin>
* 8ca9e79 - (tag: build-1.4.30-dev-2314) [KLIB] Make sure that referenced local anonymous class is deserialzied. (vor 3 Tagen) <Roman Artemev>
* 0dd329d - (tag: build-1.4.30-dev-2306) Gradle IT: Intercept and log Gradle process output on import failures (vor 4 Tagen) <Dmitriy Dolovov>
* 4de1bf8 - Gradle IT: Move GradleProcessOutputInterceptor to the test framework (vor 4 Tagen) <Dmitriy Dolovov>
* fae3ba3 - Gradle, Native: More verbose logging in KotlinToolRunner (vor 4 Tagen) <Dmitriy Dolovov>
* 734dff6 - [Commonizer] Use compact variants of collections to reduce memory footprint (vor 4 Tagen) <Dmitriy Dolovov>
* 30bf7b8 - Minor. Code formatted (vor 4 Tagen) <Dmitriy Dolovov>
* f5bb60f - [Commonizer] Refactor CIR type representation (vor 4 Tagen) <Dmitriy Dolovov>
* 2764550 - [Commonizer] Relax conditions for TA lifting-up (vor 4 Tagen) <Dmitriy Dolovov>
* cb2e94d - [Commonizer] Minor. Convert approximation keys back to data classes (vor 4 Tagen) <Dmitriy Dolovov>
* 45ce0c6 - (tag: build-1.4.30-dev-2293) avoid failing KotlinGradleIT.testInternalTest for Windows agent (vor 4 Tagen) <nataliya.valtman>
* 6d29bb5 - Fix configuration cache issue in AbstractKotlinTarget (vor 10 Tagen) <Hung Nguyen>
* 90ea64a - (tag: build-1.4.30-dev-2289) Don't report warning about changing execution order for varargs if it's inside an annotation (vor 4 Tagen) <Victor Petukhov>
* f052bc3 - Don't report warning about implicitly inferred Nothing for call arguments if there is non-Nothing return type (vor 4 Tagen) <Victor Petukhov>
* 7859287 - (tag: build-1.4.30-dev-2288) [KT-40688] Inlay Hints: lambda hints removal on editing (vor 4 Tagen) <Andrei Klunnyi>
* b3b09ea - (tag: build-1.4.30-dev-2271) FIR: adjust spec testData after DFA changes: (vor 5 Tagen) <Jinseong Jeon>
* a5389b0 - FIR DFA: use element-wise join everywhere (vor 5 Tagen) <Jinseong Jeon>
* 771c839 - FIR DFA: element-wise join at merging points of try expression (vor 5 Tagen) <Jinseong Jeon>
* bd173eb - FIR DFA: isolate effects between blocks in try expression (vor 5 Tagen) <Jinseong Jeon>
* 1f1e182 - FIR CFG: reconfigure exception throwing paths in try expression (vor 5 Tagen) <Jinseong Jeon>
* 1460360 - FIR DFA: add tests about smartcasts in/after try-catch-finally (vor 5 Tagen) <Jinseong Jeon>
* 2c8c473 - (tag: build-1.4.30-dev-2263) [JVM_IR] Enable KotlinAgainstJava and remaining JavaAgainstKotlin test. (vor 5 Tagen) <Mads Ager>
* 24345bb - (tag: build-1.4.30-dev-2262) Prevent NPE while fetching muted tests from Teamcity server (vor 5 Tagen) <Yunir Salimzyanov>
* bc2b396 - (tag: build-1.4.30-dev-2257) kotlin.native.cocoapods.targets can be a custom list of architectures (#3861) (vor 5 Tagen) <MikeKulasinski-visa>
* 0279068 - (tag: build-1.4.30-dev-2255) [JVM_IR] Follow old backend in bridge visibility and varargs marking. (vor 5 Tagen) <Mads Ager>
* 326768e - (tag: build-1.4.30-dev-2251) Minor. Update test data (vor 5 Tagen) <Ilmir Usmanov>
* bd4e2a2 - (tag: build-1.4.30-dev-2247) JVM IR: Don't generate null checks on value parameters of private operator functions (vor 5 Tagen) <Steven Schäfer>
* d4cb521 - (tag: build-1.4.30-dev-2239) JVM IR: Fix names for SAM callable references with inline class return types (vor 5 Tagen) <Steven Schäfer>
* aa81041 - (tag: build-1.4.30-dev-2235) FIR: Fix DiagnosticsTestGenerated when FIR starts requesting light classes (vor 6 Tagen) <Denis Zharkov>
* a936386 - FIR: Add more complicated workaround for OverloadResolutionByLambdaReturnType (vor 6 Tagen) <Denis Zharkov>
* 4612f26 - FIR: Add workaround for OverloadResolutionByLambdaReturnType (vor 6 Tagen) <Denis Zharkov>
* 07ed89b - Move OVERLOAD_RESOLUTION_BY_LAMBDA_ANNOTATION to compiler.common (vor 6 Tagen) <Denis Zharkov>
* a444618 - FIR: Correctly handle Java annotations from deserialization (vor 6 Tagen) <Denis Zharkov>
* f2b0d05 - (tag: build-1.4.30-dev-2231) JVM IR: minor optimizations in IrTypeMapper.classInternalName (vor 6 Tagen) <Alexander Udalov>
* b3e79d3 - (tag: build-1.4.30-dev-2223) Fix compiler warnings and some inspections (vor 6 Tagen) <Alexander Udalov>
* e5d5c20 - Minor, suppress deprecation warnings in parcelize plugin (vor 6 Tagen) <Alexander Udalov>
* ee8db2f - (tag: build-1.4.30-dev-2221) Escape resolving same library several times in K/N (#3880) (vor 6 Tagen) <LepilkinaElena>
* 1376fed - (tag: build-1.4.30-dev-2210) Support non-public inline class constructors (vor 6 Tagen) <Ilmir Usmanov>
* 88bbeea - (tag: build-1.4.30-dev-2208) Fix deprecated gradle api usage in artifact transform (vor 6 Tagen) <Bingran>
* 94145f8 - (tag: build-1.4.30-dev-2199) [IR] Inliner: supported recursion in default arguments (vor 6 Tagen) <Igor Chevdar>
* 5f8fabe - (tag: build-1.4.30-dev-2196) Minor. Update test data (vor 6 Tagen) <Ilmir Usmanov>
* 497b7ee - (tag: build-1.4.30-dev-2183) [minor] fix accidental addition of FirScriptCodegenTestGenerated (vor 6 Tagen) <Ilya Chernikov>
* 89836a2 - (tag: build-1.4.30-dev-2173) Minor, test-data-only: Remove unused "IGNORE_BACKEND_FIR" directives in asmLike test-data. (vor 6 Tagen) <Mark Punzalan>
* c02e92a - JVM IR: minor, use createJvmIrBuilder to simplify code (vor 6 Tagen) <Alexander Udalov>
* dd33ed9 - Fix suspend function with inline class types in reflection (vor 6 Tagen) <Alexander Udalov>
* 56f7e34 - (tag: build-1.4.30-dev-2171) [Gradle, K/N] Revert change of AbstractKotlinNativeCompile supertype (vor 6 Tagen) <Alexander Likhachev>
* 369b1ef - (tag: build-1.4.30-dev-2166) Fix compilation in perfTests (vor 7 Tagen) <Vladimir Dolzhenko>
* 2bffce2 - (tag: build-1.4.30-dev-2163) Revert Open build tool window on Gradle DSL errors (vor 7 Tagen) <Vladimir Dolzhenko>
* 1a57794 - (tag: build-1.4.30-dev-2162) Clean ups in IDE performance tests output (vor 7 Tagen) <Vladimir Dolzhenko>
* 6172793 - (tag: build-1.4.30-dev-2159) [JVM_IR] Rebase inline function and defaults args stepping test. (vor 7 Tagen) <Mads Ager>
* 7b315a8 - (tag: build-1.4.30-dev-2150) JVM_IR: Do not box inline class in methods (vor 7 Tagen) <Ilmir Usmanov>
* e20093d - (tag: build-1.4.30-dev-2145) Support @JvmStatic and @JvmOverload in annotation companion (vor 7 Tagen) <Mikhael Bogdanov>
* 7ec2d03 - Don't generate `final` modifier on static interface methods produced by @JvmStatic+@jvmoverloads from interface companion (vor 7 Tagen) <Mikhael Bogdanov>
@vkormushkin
Copy link
Contributor

vkormushkin commented Jul 12, 2021 •
edited
Loading

Please note that starting v1.5.30 syncFramework task will not support kotlin.native.cocoapods.target property. It will use kotlin.native.cocoapods.platform and kotlin.native.cocoapods.archs properties to produce a required framework. This is done for the sake of Apple Silicon support. As a result it will not be possible to produce a device/simulator fat framework via syncFramework task. Please consider using XCFrameworks instead, XCFrameworks support will also come with 1.5.30
@MikeKulasinski-visa FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilmat192 ilmat192 ilmat192 approved these changes

Assignees

@ilmat192 ilmat192

Labels
Native
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@MikeKulasinski-visa @crisredfi @ilmat192 @vkormushkin @abelkov @MikeKulasinski-Payworks